src/cm/lsp/serverLauncher.ts, line 583 (link)

Hardcoded Luau-specific fallback for extractFile

The default value "luau-lsp" is a Luau-specific filename used as a fallback when spec.extractFile is not set. Any future github-release installer that omits extractFile will silently generate a broken install script that tries to extract a file named luau-lsp from an unrelated archive.

The fallback should derive from the executable or binary path rather than hardcoding a server-specific string:

Even better, consider making extractFile a required field when archiveType is not "binary", since there's no sensible generic default. The validation at registration time (in serverRegistry.ts) could enforce this.

src/settings/lspServerDetail.js, line 423-454 (link)

Install/uninstall errors leave the UI state stale

installServer and uninstallServer both call throw error on failure. Since these await calls are inside the switch statement and there is no try/catch in callback, a thrown error will unwind the function before reaching refreshVisibleState on line 558. The install status row in the UI will remain unchanged after a failed operation, showing the pre-operation state indefinitely.

This can be fixed by wrapping the operations in a try/catch or by using finally:

case "install_server":
  if (!snapshot.installCommand) {
    toast("Install command not available");
    break;
  }
  try {
    await installServer(snapshot.merged, "install");
  } catch {
    // error toast is already shown by installServer
  }
  break;

The same pattern should apply to update_server and uninstall_server.

src/settings/lspSettings.js, line 1 (link)

Inconsistent API usage — serverRegistry still imported directly

lspSettings.js still imports from "cm/lsp/serverRegistry" and calls serverRegistry.listServers() on line 109, while the stated goal of this PR is to route all external access through the new lspApi. Every other settings file (lspServerDetail.js, lspConfigUtils.js, editorManager.js, acode.js) was migrated to use lspApi, leaving this file as an inconsistent outlier.

Then at line 109:

const servers = lspApi.servers.list();

src/settings/lspServerDetail.js, line 396-397 (link)

$list resolved inconsistently between show() and callback()

In the show() override (line 391), the list element is found via a specific DOM query: document.querySelector("#settings .main.list"). However, inside callback() it is resolved as this?.parentElement. These two approaches are not guaranteed to refer to the same element.

If settingsPage calls the callback with this unbound (which can happen in strict-mode arrow callbacks or if the call site doesn't set a receiver), this will be undefined and $list will be undefined. refreshVisibleState has an early-return guard for !$list, so all post-action DOM updates would silently be skipped after every user interaction.

For consistency and robustness, use the same selector as show():

src/settings/lspSettings.js, line 96-99 (link)

shell update command pre-filled with install command

In promptInstaller, the "Update Command" prompt is pre-populated with the full installCommand value:

const updateCommand = await prompt(
  "Update Command (optional)",
  String(installCommand || ""),  // pre-filled with the install command
  "textarea",
);

For shell-based installers, install and update commands are often different (e.g. the install command might delete and recreate state, while update is incremental). Pre-filling with the install command increases the risk that a user accepts it without changing it, resulting in an install script being run again during an update. A safer default would be an empty string or a short comment like # same as install.